You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The code assumes constructor will not be null when calling GetConstructor() and using ! operator. Should add null check to handle case where constructor is not found.
✅ Add null check for domain typeSuggestion Impact:The commit implements null checking in a different way by using pattern matching with 'is not null' and restructuring the code to use a switch expression that returns null for unmatched cases, achieving the same goal of preventing null reference exceptions
code diff:
+ DevToolsDomains? domains = CreateDevToolsDomain(protocolVersion, session);+ if (domains is not null)+ {+ return domains;+ }
Add null check for domainType before accessing its constructor to prevent potential NullReferenceException if no matching domain type is found.
+if (domainType == null)+ throw new ArgumentException("No matching domain type found for the specified version");
ConstructorInfo constructor = domainType.GetConstructor(new Type[] { typeof(DevToolsSession) })!;
return (DevToolsDomains)constructor.Invoke(new object[] { session });
Apply this suggestion
Suggestion importance[1-10]: 8
Why: The suggestion addresses a critical potential null reference exception that could occur if no matching domain type is found, improving the robustness of the code.
8
Add null check for attribute value
Add null check for attr before accessing its value to prevent potential NullReferenceException when processing enum members without EnumMemberAttribute.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
User description
Thanks for contributing to Selenium!
A PR well described will help maintainers to quickly review and merge it
Before submitting your PR, please check our contributing guidelines.
Avoid large PRs, help reviewers by making them as simple and short as possible.
Description
Add nullability annotations to devtools domains, as well as some event args I missed in a previous PR.
Motivation and Context
Contributes to #14640
Types of changes
Checklist
PR Type
Enhancement, Bug fix
Description
Added nullability annotations to multiple DevTools-related classes.
Improved immutability by replacing mutable properties with readonly or auto-properties.
Enhanced exception handling and safety with nullability checks and improved constructors.
Updated enum handling with conditional compilation for .NET 8.0 and above.
Changes walkthrough 📝
9 files
Added nullability annotations and improved constructor handling.Enabled nullability annotations for extension methods.Enabled nullability annotations for DevTools options.Added nullability annotations and improved immutability.Added nullability annotations and made properties readonly.Enabled nullability annotations for log message event args.Added nullability annotations and improved constructor usage.Enhanced enum handling with nullability and .NET 8.0 support.Enabled nullability annotations for response paused event args.4 files
Improved exception handling with constructor-based initialization.Improved exception handling with constructor-based initialization.Improved exception handling with constructor-based initialization.Improved exception handling with constructor-based initialization.